-
-
Notifications
You must be signed in to change notification settings - Fork 2
GH-37 Implement /withdraw command for physical banknote withdrawal #89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…k command, and implement paycheck listener
…e dependencies for improved clarity and functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a feature for players to withdraw in-game currency as physical items (checks). The implementation is mostly solid, but there are several critical and high-severity issues that need to be addressed. These include a bug that could cause players to lose items, potential data precision loss in the economy, incorrect permission setup, and a bug in argument parsing. I've also included some medium-severity suggestions to improve code quality and consistency.
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawService.java
Outdated
Show resolved
Hide resolved
...onomy-core/src/main/java/com/eternalcode/economy/command/argument/PriceArgumentResolver.java
Outdated
Show resolved
Hide resolved
...onomy-core/src/main/java/com/eternalcode/economy/command/argument/PriceArgumentResolver.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawCommand.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawService.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawTagger.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawTagger.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/EconomyBukkitPlugin.java
Outdated
Show resolved
Hide resolved
...core/src/main/java/com/eternalcode/economy/config/implementation/messages/MessageConfig.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawListener.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawManager.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawService.java
Outdated
Show resolved
Hide resolved
|
And please, please adjust to Gemini's suggestions! |
P1otrulla
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job, but you need to clean up some things.
Change spigot API for paper and remove this hacks about display name etc.
We use Bukkit#getPlayer... only in utils etc -> use normal instance of server like Server#getPlayer
Add function to block renaming check as i told you on voice chat as we're talking about it.
...leconomy-core/src/main/java/com/eternalcode/economy/command/impl/admin/AdminItemCommand.java
Outdated
Show resolved
Hide resolved
...leconomy-core/src/main/java/com/eternalcode/economy/command/impl/admin/AdminItemCommand.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawListener.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawCommand.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawCommand.java
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawManager.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawManager.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawManager.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawManager.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/EconomyPermissionConstant.java
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawCommand.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawTagger.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawTagger.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawTagger.java
Outdated
Show resolved
Hide resolved
...n/java/com/eternalcode/economy/config/implementation/messages/MessagePaycheckSubSection.java
Outdated
Show resolved
Hide resolved
...n/java/com/eternalcode/economy/config/implementation/messages/MessagePaycheckSubSection.java
Outdated
Show resolved
Hide resolved
...n/java/com/eternalcode/economy/config/implementation/messages/MessagePaycheckSubSection.java
Outdated
Show resolved
Hide resolved
...aleconomy-core/src/main/java/com/eternalcode/economy/config/implementation/PluginConfig.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawListener.java
Outdated
Show resolved
Hide resolved
…plement new withdraw functionality + Support blocking anvil interactions
…ment parsing, and enhance admin commands
…m setup logic, and improve inventory interaction messages
… to ensure prices are greater than 0.09
…ceArgumentResolver for consistency across commands
…R if currency item is AIR
...onomy-core/src/main/java/com/eternalcode/economy/command/argument/PriceArgumentResolver.java
Outdated
Show resolved
Hide resolved
...core/src/main/java/com/eternalcode/economy/config/implementation/messages/MessageConfig.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawItemService.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawMessageConfig.java
Outdated
Show resolved
Hide resolved
...-core/src/main/java/com/eternalcode/economy/withdraw/controller/WithdrawAnvilController.java
Outdated
Show resolved
Hide resolved
P1otrulla
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zrób reoznaczenie mnie jak skaczysz robić tego prka
…eImpl class for banknote handling. Add ConfigItem object to handle withdraw item properly
…figItem for better item handling and improve code readability
… of custom model data
eternaleconomy-core/src/main/java/com/eternalcode/economy/EconomyBukkitPlugin.java
Outdated
Show resolved
Hide resolved
...onomy-core/src/main/java/com/eternalcode/economy/command/argument/PriceArgumentResolver.java
Outdated
Show resolved
Hide resolved
...onomy-core/src/main/java/com/eternalcode/economy/command/argument/PriceArgumentResolver.java
Outdated
Show resolved
Hide resolved
...onomy-core/src/main/java/com/eternalcode/economy/command/argument/PriceArgumentResolver.java
Outdated
Show resolved
Hide resolved
...onomy-core/src/main/java/com/eternalcode/economy/command/argument/PriceArgumentResolver.java
Outdated
Show resolved
Hide resolved
...core/src/main/java/com/eternalcode/economy/config/implementation/messages/MessageConfig.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawItemServiceImpl.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawItemServiceImpl.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawService.java
Outdated
Show resolved
Hide resolved
…h MoneyFormatArgument for improved money value parsing
…dability and consistency
eternaleconomy-core/src/main/java/com/eternalcode/economy/config/item/ConfigItem.java
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/config/item/ConfigItem.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/config/item/ConfigItem.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawItemServiceImpl.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawItemServiceImpl.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawItemServiceImpl.java
Outdated
Show resolved
Hide resolved
eternaleconomy-core/src/main/java/com/eternalcode/economy/withdraw/WithdrawItemServiceImpl.java
Outdated
Show resolved
Hide resolved
imDMK
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job
CitralFlo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rollback changes regarding Argument
No description provided.